-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make AsRef and AsMut reflexive #39397
Conversation
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
89733c3
to
4a9884d
Compare
Unfortunately, this is why: https://travis-ci.org/rust-lang/rust/jobs/196373035#L384
|
Seems problematic per @Stebalien. |
I remember someone asking why we don't have this impl in one of the IRC channels the other day and we also guessed it would conflict with precisely that existing implementation. Is this something that might become possible after specialization stabilizes? |
Sometimes I wish that there were a trait similar to:
Such that I could provide |
Should this be closed? |
A great read on why this can't be done and maybe one day we will be able to do it: |
I'll close this for now and re-open when impl specialisation hits. |
Can't the conflicting implementations above be removed? I guess specialization is only necessary to prevent downstream crates from conflicting with the new generic implementation, right?? Link: tracking issue for specialization (RFC 1210). I would like to make Here is my use case :-) let opt: &Option<String>;
// ...
let result: &str = opt // &Option<String>
.as_ref() // Option<&String>
.map(|x| x.as_ref()) // Option<&str> -- this step could be omitted
.unwrap_or(".. nothing there"); // &str |
@colin-kiegel To be clear we need more than specialization as it exists to make this work, so stabilizing specialization and closing that issue is not enough. If the other impls are removed then we lose functionality because the impls intersect but one is not contained in the other. |
…riplett docs: Improve AsRef / AsMut docs on blanket impls There are several issues with the current state of `AsRef` and `AsMut` as [discussed here on IRLO](https://internals.rust-lang.org/t/semantics-of-asref/17016). See also rust-lang#39397, rust-lang#45742, rust-lang#73390, rust-lang#98905, and the FIXMEs [here](https://github.com/rust-lang/rust/blob/1.62.0/library/core/src/convert/mod.rs#L509-L515) and [here](https://github.com/rust-lang/rust/blob/1.62.0/library/core/src/convert/mod.rs#L530-L536). These issues are difficult to fix. This PR aims to update the documentation to better reflect the status-quo and to give advice on how `AsRef` and `AsMut` should be used. In particular: - Explicitly mention that `AsRef` and `AsMut` do not auto-dereference generally for all dereferencable types (but only if inner type is a shared and/or mutable reference) - Give advice to not use `AsRef` or `AsMut` for the sole purpose of dereferencing - Suggest providing a transitive `AsRef` or `AsMut` implementation for types which implement `Deref` - Add new section "Reflexivity" in documentation comments for `AsRef` and `AsMut` - Provide better example for `AsMut` - Added heading "Relation to `Borrow`" in `AsRef`'s docs to improve structure
…riplett docs: Improve AsRef / AsMut docs on blanket impls There are several issues with the current state of `AsRef` and `AsMut` as [discussed here on IRLO](https://internals.rust-lang.org/t/semantics-of-asref/17016). See also rust-lang#39397, rust-lang#45742, rust-lang#73390, rust-lang#98905, and the FIXMEs [here](https://github.com/rust-lang/rust/blob/1.62.0/library/core/src/convert/mod.rs#L509-L515) and [here](https://github.com/rust-lang/rust/blob/1.62.0/library/core/src/convert/mod.rs#L530-L536). These issues are difficult to fix. This PR aims to update the documentation to better reflect the status-quo and to give advice on how `AsRef` and `AsMut` should be used. In particular: - Explicitly mention that `AsRef` and `AsMut` do not auto-dereference generally for all dereferencable types (but only if inner type is a shared and/or mutable reference) - Give advice to not use `AsRef` or `AsMut` for the sole purpose of dereferencing - Suggest providing a transitive `AsRef` or `AsMut` implementation for types which implement `Deref` - Add new section "Reflexivity" in documentation comments for `AsRef` and `AsMut` - Provide better example for `AsMut` - Added heading "Relation to `Borrow`" in `AsRef`'s docs to improve structure
…riplett docs: Improve AsRef / AsMut docs on blanket impls There are several issues with the current state of `AsRef` and `AsMut` as [discussed here on IRLO](https://internals.rust-lang.org/t/semantics-of-asref/17016). See also rust-lang#39397, rust-lang#45742, rust-lang#73390, rust-lang#98905, and the FIXMEs [here](https://github.com/rust-lang/rust/blob/1.62.0/library/core/src/convert/mod.rs#L509-L515) and [here](https://github.com/rust-lang/rust/blob/1.62.0/library/core/src/convert/mod.rs#L530-L536). These issues are difficult to fix. This PR aims to update the documentation to better reflect the status-quo and to give advice on how `AsRef` and `AsMut` should be used. In particular: - Explicitly mention that `AsRef` and `AsMut` do not auto-dereference generally for all dereferencable types (but only if inner type is a shared and/or mutable reference) - Give advice to not use `AsRef` or `AsMut` for the sole purpose of dereferencing - Suggest providing a transitive `AsRef` or `AsMut` implementation for types which implement `Deref` - Add new section "Reflexivity" in documentation comments for `AsRef` and `AsMut` - Provide better example for `AsMut` - Added heading "Relation to `Borrow`" in `AsRef`'s docs to improve structure
This would probably require a crater run.
I found it weird that
T
didn't implementAsRef
andAsMut
, considering how the conversions are trivial.